Allow tags or indices#1
Conversation
…ow integer as well as string inputs
garryod
left a comment
garryod
left a comment
There was a problem hiding this comment.
Looks good overall, only real thought is that I reckon it'd be nice if the rest API took separate tag and idx fields, perhaps with a structure akin to:
class DeleteParams(BaseModel):
tag: Optional[str]
idx: Optional[int]
@property ref(self) -> Union[int, str]:
...
|
I thought about doing this, the only problem is then I'd have to add extra validation to make sure the user definitely input either an index or a tag. Also, Irakli's functions that I call have just one parameter for this, which acts as both. |
It would definitely add a bit of polish if you were to, but it's just that. Might be worth having a look at Irakli's code to see why this is done and if it would be better separated into two functions (one which takes an |
3 participants
Some of my routes in routes/UBCalculation.py delete or edit orientations/reflections in a pickled HklCalculation object. In Irakli's code, this can be done with both a tag for the desired orientation/reflection, and an index.
I have made changes to do the same here, so that the client using this API can request for an index in the list of orientations/reflections to be deleted, OR for a specific tag of orientations/reflections to be deleted. This worked with editing orientations/reflections because the tagOrIdx parameter was wrapped in a pydantic model - I've since realised that I needed to do the same for my delete routes as well, so I have made a simple deleteParams pydantic model to hold this information.
As a result, some tests have also changed. I have deleted the test_diffcalc_API.py file as well, as this was a dummy file.